Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New module: Add Openstack Tempest run module (cloud/openstack/os_tempest_run) #24173

Closed
wants to merge 5 commits into from

Conversation

TalShafir
Copy link

SUMMARY

New module that lets the user run Tempest

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

os_tempest_run

ANSIBLE VERSION
ansible 2.4.0 (module/os_tempest_run c7c81835cb) last updated 2017/05/01 21:11:35 (GMT +300)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/tshafir/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/tshafir/ansible/lib/ansible
  executable location = /home/tshafir/ansible/bin/ansible
  python version = 2.7.5 (default, Aug 18 2016, 15:58:25) [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)]

@ansibot
Copy link
Contributor

ansibot commented May 1, 2017

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/modules/cloud/openstack/os_tempest_run.py:28:78: W291 trailing whitespace
lib/ansible/modules/cloud/openstack/os_tempest_run.py:79:145: W291 trailing whitespace
lib/ansible/modules/cloud/openstack/os_tempest_run.py:236:1: W293 blank line contains whitespace

click here for bot help

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 ci_verified Changes made in this PR are causing tests to fail. cloud module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. new_plugin This PR includes a new plugin. openstack labels May 1, 2017
@TalShafir
Copy link
Author

TalShafir commented May 1, 2017

The bot didn't tag anyone again.
@emonty @Shrews @juliakreger @j2sol @rcarrillocruz @Thingee

ready_for_review

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 1, 2017
options:
workspace:
description:
The workspace as was configured in 'Tempest init <I(workspace)>'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The I(workspace) will turn into italics, which I'm not sure if that's what you want in this case

https://docs.ansible.com/ansible/dev_guide/developing_modules_documenting.html#formatting-options

Copy link
Author

@TalShafir TalShafir May 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says "I() for option names" but I think I'll change that to C(workspace) to make it more visible.
edit: now that I think about it, it should be C(workspace) as it is the option value and not the name.

notes:
- You can find out more about Tempest at U(http://docs.openstack.org/developer/tempest/)
- The module requires to tempest init <I(workspace)> before usage
- The options I(whitelist_file) and I(blacklist_file) are mutually exclusive, if they are both given only the I(whitelist_file) will be used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the mutually_exclusive function you can remove this line

'''

EXAMPLES = '''
# Run all tests with default number of workers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per Ansible best practice, please use - name: Run all tests with default number of workers then remove the - from in front of the module name

http://docs.ansible.com/ansible/dev_guide/developing_modules_documenting.html#examples-block

Copy link
Author

@TalShafir TalShafir May 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I fixed it.
I originally wrote it after looking at the pip module as an example (https://docs.ansible.com/ansible/pip_module.html#examples), might be nice to add it as an issue for something like 'First Timers Only' or 'YourFirstPR'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

Where in the existing documentation/examples did you look?

I'm wondering where I can put/link to the example so people will find it.

Copy link
Author

@TalShafir TalShafir May 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concurrency=dict(type="int", required=False, default=None),
force=dict(type="bool", required=False, default=False),
subunit=dict(type="bool", required=False, default=True),
))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some options to AnsibleModule which you may be able to use to validate options passed in.
Look at existing modules for examples:

mutually_exclusive
required_together
required_one_of
require_if

Should whitelist_file and blacklist_file be mutually_exclusive
mutually_exclusive=(('blacklist_file', 'whitelist_file'),),

DOCUMENTATION = '''
---
module: os_tempest_run
short_description: Runs Tempest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it might be worth saying Run OpenStack Tempest to make it a little easier for people to find via a search engine.

:arg follow: A boolean to indicate of symlinks should be resolved or not
:raises UnicodeDecodeError: If the canonicalized version of the path
contains non-utf8 byte sequences.
:rtype: A text string (unicode on pyyhon2, str on python3).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pyyhon2 -> python2

@gundalow gundalow removed the needs_triage Needs a first human triage before being processed. label May 3, 2017
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels May 3, 2017
@TalShafir
Copy link
Author

Thanks for the review @gundalow
ready_for_review

@ansibot
Copy link
Contributor

ansibot commented May 3, 2017

waiting_on: TalShafir
module: os_tempest_run
changes_requested_by: gundalow
needs_info: False
needs_revision: True
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits: 0
community_shipits: 0
ansible_shipits: 0
shipit_actors: []

click here for bot help

@TalShafir
Copy link
Author

ready_for_review

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 15, 2017
@alikins alikins changed the title New Module: os_tempest_run module New module: Add Cloudstack Tempest run module (cloud/openstack/os_tempest_run) May 22, 2017
@alikins alikins changed the title New module: Add Cloudstack Tempest run module (cloud/openstack/os_tempest_run) New module: Add Openstack Tempest run module (cloud/openstack/os_tempest_run) May 22, 2017
@Shrews
Copy link
Contributor

Shrews commented May 24, 2017

Hi! Thanks for the new module. However, I don't feel like tempest modules are a good fit here. The current set of modules are geared toward using OpenStack services. Modules for testing the services seems like they should be kept separate. Most consumers of the core Ansible OpenStack cloud modules are only going to be interested in the former, not the latter.

And on a more personal level, as a core maintainer for these modules, I don't want to have to learn the tempest testing environment in order to support these going forward. I just have no interest in it.

I'd like to get more feedback from possibly @emonty, @j2sol, or @rcarrillocruz and get their thoughts on this.

@omgjlk
Copy link
Contributor

omgjlk commented May 24, 2017

I agree with @Shrews

@ansibot
Copy link
Contributor

ansibot commented Jun 27, 2017

@j2sol @juliakreger @Shrews @Thingee

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@ansibot ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Jun 29, 2017
@ansibot
Copy link
Contributor

ansibot commented Sep 1, 2017

@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Jan 22, 2018
@ansibot
Copy link
Contributor

ansibot commented Oct 13, 2018

cc @mnaser
click here for bot help

@mnaser
Copy link
Contributor

mnaser commented Oct 13, 2018

I agree with @omgjlk and @Shrews. I would recommend the following role to run Tempest if you'd like

https://github.com/openstack/openstack-ansible-os_tempest

@emonty do you feel the same as well?

@ansibot
Copy link
Contributor

ansibot commented Nov 2, 2018

@ansibot
Copy link
Contributor

ansibot commented Apr 6, 2019

cc @gtema
click here for bot help

@sshnaidm
Copy link
Contributor

Thanks for submitting patch for Openstack Ansible modules!
We moved Openstack Ansible modules to Openstack repositories.
Next patches should be submitted not with Ansible Github but with
Openstack Gerrit: https://review.opendev.org/#/q/project:openstack/ansible-collections-openstack
Please submit your code there from now.
Thanks for your contribution and sorry for inconvienience.

@ansibot ansibot added collection Related to Ansible Collections work collection:openstack.cloud needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md labels Apr 29, 2020
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed collection:openstack.cloud community_review In order to be merged, this PR must follow the community review workflow. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community. labels Jul 16, 2020
@Akasurde
Copy link
Member

Closing as per above.

@Akasurde Akasurde closed this Aug 19, 2020
@ansible ansible locked and limited conversation to collaborators Sep 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 cloud collection Related to Ansible Collections work core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. new_module This PR includes a new module. new_plugin This PR includes a new plugin. openstack stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants